The regex-based reference extraction and the JSON-based collectRscReferenceIds overlap: references inside a JSON payload (like ["$L5","$L2"]) will be found by both the regex scan and the JSON.parse → collectRscReferenceIds path. This is harmless since into is a Set (idempotent adds), but the regex scan does redundant work for JSON-carrying rows.

Not a correctness issue — just noting the double-pass in case you later want to optimize the hot path. The regex path is necessary for non-JSON rows like 1:D"$3a", so the two can't simply be collapsed.

Also: the negative lookbehind (?<!\$) prevents matching $$5 (literal-dollar escape), but it won't prevent matching inside a JSON string value like "$$5" where the $ is a literal character. In practice this would only cause a false-positive live-id addition (keeping an extra row alive), which is safe — it errs on the side of over-inclusion rather than dropping rows.

Left this behavior unchanged. The regex pass is still needed for non-JSON rows, and the JSON walk remains the simpler way to keep nested references honest. Since both paths feed a Set, the overlap stays idempotent; if this ever shows up in profiling, I’d optimize it as a separate hot-path pass rather than folding more heuristics into this PR.

The REFERENCE_PATTERN character class [LBFQWK@] matches the current React 19.2.x tags. The comment rightly notes this needs auditing on React upgrades. Consider adding a test (or a comment linking to the specific React source location) that documents which React Flight tags are reference-like vs. escape-like, so the next person upgrading React knows exactly what to check.

For reference, the React source file is packages/react-client/src/ReactFlightClient.js — the resolveModel / tag switch statement.

Added a source breadcrumb next to REFERENCE_PATTERN pointing at React’s ReactFlightClient.js tag parser so the next React upgrade has an explicit audit target.

Minor: promoteToStreaming hardcodes 0: as the row 0 prefix. If React were to ever emit row 0 with a different hex representation (e.g., 00:), this would produce a different prefix than the original. Extremely unlikely given React's wire format, but since you already have the original row 0's raw prefix available, you could preserve it:

This is a nitpick — 0: is the correct canonical form.

Fixed. promoteToStreaming now preserves the original row-0 prefix from row0Raw instead of hardcoding 0:, and there’s a focused test covering a 00: row id form.

The header parsing accepts any layout:* prefixed value without length or character validation. A malicious client could send a very long header with many entries to force the server to allocate a large Set. Since this runs on every RSC request (when the header is present), consider adding a cap — e.g., bail out or truncate after N entries (say 50, since deeply nested layout trees beyond that would be unusual).

This is low severity since Cloudflare Workers already enforce request header size limits, but defense-in-depth at the application layer doesn't hurt.

Fixed. parseSkipHeader now uses a bounded parser and stops after 50 unique layout:* ids, which keeps the allocation surface capped even if a client sends a pathological skip header. Added a test for the cap.

When isRscRequest is false, this allocates a fresh new Set() on every non-RSC request. This is fine since it's a tiny allocation, but for consistency with the EMPTY_SKIP_SET pattern used in app-page-render.ts and app-page-cache.ts, you could use a shared sentinel. Not blocking — the generated entry intentionally avoids importing runtime helpers it doesn't need, and this allocation is negligible.

Fixed. The generated RSC entry now uses a shared empty skip set for the non-RSC path instead of allocating a fresh new Set() per request. Updated the entry snapshots along with the template change.

Good use of EMPTY_SKIP_SET as the fallback — wrapRscBytesForResponse short-circuits on empty set and returns the raw ArrayBuffer without creating a ReadableStream. The cache-hit hot path stays allocation-free when no skip header is present.

Worth noting for future readers: rscForResponse is also used for the HTML SSR path (line 261 in this file), but skipIds will always be empty when !isRscRequest, so the HTML path always receives the unfiltered stream. The code flow is correct; a brief inline comment at the HTML usage site could save someone from a double-take.

Added an inline comment at the HTML SSR call site clarifying that rscForResponse is still canonical there because skipIds only becomes non-empty for RSC requests.

computeSkipDecision allocates a new Set<string>() even when no requested ids match any "s" flags (i.e., the client asked to skip layouts but the server classified all of them as dynamic). The function returns this empty Set instead of EMPTY_SKIP_DECISION.

This means skipIds.size > 0 downstream is still false so it's not a correctness bug, but it defeats the purpose of the EMPTY_SKIP_DECISION sentinel since the caller (renderAppPageLifecycle) checks skipIds.size > 0 to decide whether to create a transform stream. Consider:

Fixed. computeSkipDecision now returns the shared empty sentinel when every requested id is rejected, and there’s a focused test that checks the all-dynamic case reuses the same empty reference instead of leaking a fresh Set.

The previous review flagged that this allocates a fresh new Set() on every non-RSC request. The author responded that this was fixed to use a shared empty sentinel, but the code still shows const __EMPTY_SKIP_LAYOUT_IDS = new Set() inside the request handler body — meaning it's allocated per-request, not shared.

To actually share it, this needs to be hoisted outside _handleRequest to module scope in the generated entry (similar to how EMPTY_SKIP_SET is a module-level constant in app-page-render.ts and app-page-cache.ts). The current placement inside the function body means a new Set instance is created on every request, including non-RSC ones where __skipLayoutIds immediately points to it.

Not a correctness issue (empty Set is cheap), but the intent from the review response doesn't match what landed.

Fixed. The empty skip set is now hoisted to module scope in the generated RSC entry template, so non-RSC requests really reuse a shared sentinel instead of allocating inside _handleRequest. Updated the entry snapshots with the hoist.

Nit: the inline regex here duplicates the tag character class from REFERENCE_PATTERN on line 32. If a new React Flight tag is added and only one pattern is updated, the filter would silently diverge between the JSON walk path and the raw-regex path.

Consider extracting the tag class to a shared constant, e.g.:

const FLIGHT_REF_TAGS = "LBFQWK@";
const REFERENCE_PATTERN = new RegExp(`^\\$(?:[${FLIGHT_REF_TAGS}])?([0-9a-fA-F]+)$`);

and reusing FLIGHT_REF_TAGS in the matchAll regex on this line. Not blocking, but reduces the audit surface on React upgrades.

Applied. The Flight reference tags now live in a shared FLIGHT_REF_TAGS constant, and both REFERENCE_PATTERN and the raw matchAll regex derive from it. That keeps the JSON-walk path and raw-row path aligned for future React upgrades.

Worth noting: when row 0 arrives, promoteToStreaming may replace state from initial to streaming (or passthrough on JSON parse failure). The consumeBuffered loop continues iterating the same buffer and the next iteration falls through to the streaming/passthrough branch, which is correct. But if promoteToStreaming enters the JSON-parse-failure fallback (state = { phase: "passthrough" }), the carry from the initial phase is already empty (line 255 clears state.pending), yet the new passthrough state's carry starts at "". This is fine because consumeBuffered sets state.carry at the end of the loop, but it took me a few reads to verify. The docblock on line 223 helps — just flagging this as the most subtle control flow in the file.

The ?? true default for supportsFilteredRscStream means the filter is opt-out rather than opt-in. Since the generated entry currently always passes false, this is safe today. But if a future caller forgets to set the flag, the filter will activate by default.

Given that this is explicitly "dormant until canonical-stream is validated" per the PR description, an opt-in default (?? false) would be more conservative and match the stated intent. When PR 5/6 or 6/6 is ready to activate the filter, it would explicitly set true.

This way, omitting the flag keeps the filter off rather than on.

Applied. The dormant filter is now opt-in: supportsFilteredRscStream defaults to false in renderAppPageLifecycle, and the render tests now prove that omitting the flag keeps the response canonical until a later PR explicitly enables it.

The cache-read path applies skipIds to the cached canonical bytes here. This is correct for the response sent to the current client, but I want to confirm: the skipIds used here come from the current request's X-Vinext-Router-Skip header (threaded from the generated entry → readAppPageCacheResponse → buildAppPageCachedResponse), not from the request that originally populated the cache. This is the right behavior — each client gets a response filtered to its own skip set — just want to make sure this is documented as intentional since it could be confusing during debugging (same cache entry, different response bytes for different clients).

Documented. Added an inline comment on the cache-read path clarifying that the cached bytes stay canonical and the current request’s skipIds only shape the egress response for this client. That matches the intended canonical-cache / per-client-response split.

Nit: missing blank line between the closing }); of the previous describe block and this new describe. Every other top-level describe in this file has a blank separator.

Nit: when consumeBuffered is called from flush with buffer + "\n", and row 0 arrives as the final line, promoteToStreaming replaces state and the loop continues to the next iteration. At that point cursor === buffer.length, so the while-loop exits and we fall through to state.carry = "". This clears the new streaming state's carry, which is correct but only because the synthesized newline means there's never trailing content after the last row.

This is fine — just noting it as the subtlest control flow in the file. The docblock on line 228 helps.

This feels rather fragile and could be rather problematic if people want to use their own versions, so I don't really think this is an approach we should rely on...

Is there another way we can achieve what we're trying to do without that?

I don't think any of the reference frameworks hack around the wire format response - I believe they would filter before it gets to this point. Given that people can bring their own versions of React instead of vendoring it, this would be very risky.

Wouldn't it be possible for this to potentially throw up false positives from scanning the payload?

So from what I gather, it sounds like we render the entire tree, then skip returning parts of that tree to client? Wouldn't we want to skip rendering those before-hand to reduce server work? I believe both Next.js and Waku would skip before the rendering.